Hi, I think you're not handling correctly the flag for non-string value

I recommend you to add test for

filter_var(true, options:FILTER_FLAG_EMPTY_STRING_NULL); // non-empty-string
filter_var(false, options:FILTER_FLAG_EMPTY_STRING_NULL); // null
filter_var($bool, options:FILTER_FLAG_EMPTY_STRING_NULL); // null
filter_var(0.0, options:FILTER_FLAG_EMPTY_STRING_NULL); // non-empty-string
filter_var(0, options:FILTER_FLAG_EMPTY_STRING_NULL); // non-empty-string
filter_var(null, options:FILTER_FLAG_EMPTY_STRING_NULL); // null
filter_var($nullable, options:FILTER_FLAG_EMPTY_STRING_NULL); // null|non-empty-string

You certainly need to do the check you did but after a toString call.

Thanks for your feedback! I added some cases and tried to be as precise as I could on type expectations. I feel like it's starting to get too complex when going deeper on union types. I'm not feeling fluent enough with PHPStan's API but I did my best.

Again, tell me if I got something wrong here 😉

I feel like it could be simpler to look for something like

if ($filterValue === $this->getConstant('FILTER_DEFAULT')) {
             $scalarOrNull = new UnionType([
				new StringType(),
				new FloatType(),
				new BooleanType(),
				new IntegerType(),
				new NullType(),
			]);
			if ($scalarOrNull->isSuperTypeOf($in)->yes()) {
				$canBeSanitized = $this->canStringBeSanitized($filterValue, $flagsType);
				if ($canBeSanitized->no()) {
					$stringType = $in->toString();
				} else {
					$stringType = $in->isString()->no()
						? $in->toString()
						: TypeCombinator::union(TypeCombinator::remove($in, new StringType()), new StringType());
				}

				return $this->handleEmptyStringNullFlag($stringType, $flagsType);
			}
		}

with

private function handleEmptyStringNullFlag(Type $in, ?Type $flagsType): Type
	{
		$hasFlag = $this->hasFlag('FILTER_FLAG_EMPTY_STRING_NULL', $flagsType);
		if ($hasFlag->no()) {
			return $in;
		}

		$hasEmptyString = !$in->isSuperTypeOf(new ConstantStringType(''))->no();
		if ($hasFlag->maybe()) {
			return $hasEmptyString ? TypeCombinator::addNull($in) : $in;
		}

		return $hasEmptyString ? TypeCombinator::remove(TypeCombinator::addNull($in), new ConstantStringType('')) : $in;
	}

Some existing tests will fail, but I think it's an improvement.
There will be only

		$return = filter_var($nullable_string, options: FILTER_FLAG_EMPTY_STRING_NULL);
		assertType('non-empty-string|null', $return);

to fix. This is something related to (I think)

if ($exactType === null || $hasOptions->maybe() || (!$inputType->equals($type) && $inputType->isSuperTypeOf($type)->yes())) {
			if (!$defaultType->isSuperTypeOf($type)->yes()) {
				$type = TypeCombinator::union($type, $defaultType);
			}
		}

I think you could try something like https://github.com/phpstan/phpstan-src/compare/2.1.x...VincentLanglet:phpstan-src:fix/filter_var?expand=1 with all your existing tests @niconoe-

Thank you very much for your feedback!

I'm giving it a try right now.

I'm just curious about this snippet:

$stringType = $in->isString()->no()
    ? $in->toString()
    : TypeCombinator::union(TypeCombinator::remove($in, new StringType()), new StringType());

In the else part of the ternary, I can't read something else than "Return a union of input type minus 'string', plus 'string'." which is basically the input type itself. Am I missing something?

EDIT: nvm, I got it. Aim is to remove any kind of StringType more specific than just a StringType, like an AccessoryNonEmptyStringType for instance, and replace it by a general StringType. This is due to possible sanitations when using flags that will remove unwanted chars from a non-empty-string that could lead to empty strings, and as we don't know, we infer the general StringType.
Got it ! 😄

I tried your proposal and I got 141 errors because of LegacyNodeScopeResolverTest, like for instance, filter_var($mixed, FILTER_DEFAULT) is expected to return string|false but know is infered as returning string.

I think this is due to

if ($canBeSanitized->no()) {
	$stringType = $in->toString();
}

Forcing the input type as a String only matters when having the flag FILTER_FLAG_EMPTY_STRING_NULL (at least, for the test cases I want to fix), IMO, so doing this change outside of the context of using such flag breaks the general usecases.
I'll try to get a fix about it.

I tried your proposal and I got 141 errors because of LegacyNodeScopeResolverTest, like for instance, filter_var($mixed, FILTER_DEFAULT) is expected to return string|false but know is infered as returning string.

It's certainly because of the if ($this->isValidationFilter($filterValue)) { I tried to add, but I was wrong since something like
filter_var(new \DateTime()) is false without a ValidationFilter

Maybe you'll have less failure to fix with https://github.com/phpstan/phpstan-src/compare/2.1.x...VincentLanglet:phpstan-src:fix/filter_var_2?expand=1

It's certainly because of the if ($this->isValidationFilter($filterValue)) { I tried to add, but I was wrong since something like filter_var(new \DateTime()) is false without a ValidationFilter

I figured this out too, but I kept the new method isValidationFilter as it is still used more than once, even when removing its call around

if ($exactType === null || $hasOptions->maybe() || (!$inputType->equals($type) && $inputType->isSuperTypeOf($type)->yes())) {
			if (!$defaultType->isSuperTypeOf($type)->yes()) {
				$type = TypeCombinator::union($type, $defaultType);
			}
		}

I had to fix the remaining failling unit test about filter_var($nullable_string, options: FILTER_FLAG_EMPTY_STRING_NULL); with a special case, as I don't want to add the default type when using FILTER_FLAG_EMPTY_STRING_NULL and input type is null|string.

I really don't understand why the unit tests are not passing on PHP 7.4 only and why the actual results differ from the expectations. The behavior of filter_var remained unchanged AFAIK.

For the other errors on the pipeline, there are issues about things I did not impact, and memory limit troubles. For both of them, I don't know if I have something to do.

About Mutation testing, I don't get how could I kill both kind of mutations like this:

- if (!$ternaryLogic->yes()) {
+ if ($ternaryLogic->no()) {

and

- if ($ternaryLogic->no()) {
+ if (!$ternaryLogic->yes()) {

when $ternaryLogic cannot be "maybe".

Therefore, I don't think such mutation is relevant IMO.

And about other failing tests, this is about memory_limit so, I'm not sure I could do something to be fair.

For if ($scalarOrNull->isSuperTypeOf($in)->yes()) you can test the maybe case with something like
filter_var($stringOrObject) I think (or filter_var($mixed) ?)

Maybe some of the other condition can be checked the same way.
I feel like you're getting some mutation testing on some trinary you didn't originally write, so I would focus on the code you introduced first.

I added unit tests with all combinations of types I got on this unit test file and I added mixed too. I'm getting some unexpected results because as soon as the input might be an object, the analysis of the flag is not done and default return type string|false is given.

I'm gonna need some time to fix this.

@VincentLanglet I added unit tests with all types and anyOf all types + mixed, which helped into getting even more precise retun types, but some mutants are still alive. I'm not sure how I could kill them by tests.
Probably the code should be optimized in a way I'm not confindent enough to handle it, TBH.

Do you have any ideas about it?

Thanks a lot!

Do you have any ideas about it?

I'm not fully familiar with all those mutation testing errors, I think this is something introduced by @staabm.
I sometimes find the report are false-positive or unpossible case. But maybe he could help you on it

Is there a way to run the mutation testing locally so I could give it a try to find how to kill the mutants? I really wish this PR not to be ignored because of this 😅

Any advice you could give @staabm ?

Thanks a lot!